Skip to content

Conversation

@denrase
Copy link
Collaborator

@denrase denrase commented Nov 26, 2025

📜 Description

  • Call captureLogs on batcher if app terminates
  • Call captureLogs on batcher if app resigns active

Introduces listeners on the AppState and observes it in a conditional (UIKit) integration.

💡 Motivation and Context

Implements missing behaviour from batch processor spec.

When the application shuts down gracefully, the BatchProcessor SHOULD forward all data in memory to the transport. The transport SHOULD keep its existing behavior, which usually stores the data to disk as an envelope. It is not required to call a transport flush. This is mostly relevant for mobile SDKs already subscribed to these hooks, such as applicationWillTerminate on iOS.

When the application moves to the background, the BatchProcessor SHOULD forward all the data in memory to the transport and stop the timer. The transport SHOULD keep its existing behavior, which usually stores the data to disk as an envelope. It is not required to call the transport flush. This is mostly relevant for mobile SDKs.

Closes #6478

💚 How did you test it?

Added tests and tested manually.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

@denrase denrase changed the title Flush Logs on Terminate or Resign Active State Flush Logs on WillTerminate or WillResignActive` App State Nov 26, 2025
@denrase denrase changed the title Flush Logs on WillTerminate or WillResignActive` App State Flush Logs on WillTerminate or WillResignActive App State Nov 26, 2025
@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 93.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.082%. Comparing base (87a7e98) to head (0f74c74).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
Sources/Sentry/SentryBaseIntegration.m 33.333% 2 Missing ⚠️
.../Swift/Integrations/Log/FlushLogsIntegration.swift 95.000% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6909       +/-   ##
=============================================
+ Coverage   85.057%   85.082%   +0.024%     
=============================================
  Files          453       454        +1     
  Lines        27633     27686       +53     
  Branches     12139     12155       +16     
=============================================
+ Hits         23504     23556       +52     
+ Misses        4086      4084        -2     
- Partials        43        46        +3     
Files with missing lines Coverage Δ
SentryTestUtils/Sources/TestClient.swift 85.321% <100.000%> (+0.415%) ⬆️
Sources/Sentry/SentryClient.m 97.623% <100.000%> (+0.206%) ⬆️
Sources/Swift/AppState/SentryAppState.swift 95.945% <ø> (ø)
Sources/Swift/AppState/SentryAppStateManager.swift 100.000% <ø> (ø)
Sources/Swift/Core/Integrations/Integrations.swift 93.750% <100.000%> (+0.892%) ⬆️
Sources/Swift/SentryDependencyContainer.swift 98.496% <ø> (ø)
Sources/Sentry/SentryBaseIntegration.m 94.214% <33.333%> (-1.548%) ⬇️
.../Swift/Integrations/Log/FlushLogsIntegration.swift 95.000% <95.000%> (ø)

... and 10 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87a7e98...0f74c74. Read the comment docs.

@denrase
Copy link
Collaborator Author

denrase commented Nov 26, 2025

The app state manager is only enabled for UIKit, but the notification-equivalents for terminate/active should also be available for macOS. Is this intended behaviour?

@denrase denrase added the ready-to-merge Use this label to trigger all PR workflows label Nov 26, 2025
@denrase denrase marked this pull request as ready for review November 26, 2025 10:25
@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1231.78 ms 1268.67 ms 36.89 ms
Size 24.14 KiB 1.01 MiB 1013.68 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d38165b 1211.41 ms 1242.49 ms 31.08 ms
409a607 1229.57 ms 1251.45 ms 21.88 ms
94a6b1a 1213.39 ms 1231.55 ms 18.17 ms
7f4bf81 1241.73 ms 1270.66 ms 28.93 ms
4d264fa 1223.48 ms 1246.91 ms 23.44 ms
c2982e7 1223.92 ms 1242.86 ms 18.94 ms
d66f082 1227.08 ms 1247.04 ms 19.96 ms
e36128b 1213.04 ms 1248.60 ms 35.56 ms
5db87fa 1218.88 ms 1251.53 ms 32.65 ms
5c2d5d6 1225.39 ms 1259.45 ms 34.06 ms

App size

Revision Plain With Sentry Diff
d38165b 23.75 KiB 855.37 KiB 831.62 KiB
409a607 23.74 KiB 874.08 KiB 850.33 KiB
94a6b1a 23.75 KiB 902.48 KiB 878.74 KiB
7f4bf81 23.75 KiB 919.70 KiB 895.95 KiB
4d264fa 23.74 KiB 874.07 KiB 850.33 KiB
c2982e7 23.75 KiB 911.62 KiB 887.87 KiB
d66f082 23.75 KiB 928.85 KiB 905.10 KiB
e36128b 23.75 KiB 963.19 KiB 939.44 KiB
5db87fa 23.75 KiB 926.65 KiB 902.90 KiB
5c2d5d6 23.75 KiB 969.28 KiB 945.54 KiB

Previous results on branch: feat/flush-logs-on-app-state-change

Startup times

Revision Plain With Sentry Diff
58c01c4 1193.19 ms 1242.47 ms 49.28 ms
5a96fbe 1223.24 ms 1239.82 ms 16.57 ms
e1bf267 1215.20 ms 1241.04 ms 25.84 ms
6b6ef93 1192.06 ms 1212.08 ms 20.02 ms

App size

Revision Plain With Sentry Diff
58c01c4 24.14 KiB 1.01 MiB 1013.67 KiB
5a96fbe 24.14 KiB 1.01 MiB 1015.46 KiB
e1bf267 24.14 KiB 1.01 MiB 1015.57 KiB
6b6ef93 24.14 KiB 1.01 MiB 1015.51 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. I found a few high-level issues we need to address before I give this a closer look.

- Expose attachment type on `SentryAttachment` for downstream SDKs (like sentry-godot) (#6521)
- Increase attachment max size to 100MB (#6537)
- Increase maximum attachment size to 200MB (#6726)
- Flush Logs on `WillTerminate` or `WillResignActive` App State (#6909)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 🚫 The changelog entry seems to be part of an already released section ## 9.0.0.
    Consider moving the entry to the ## Unreleased section, please.

#elseif (os(macOS) || targetEnvironment(macCatalyst)) && !SENTRY_NO_UIKIT
import AppKit
private typealias CrossPlatformApplication = NSApplication
#endif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Mac Catalyst incorrectly imports AppKit instead of UIKit

The conditional compilation directives incorrectly import AppKit for Mac Catalyst apps. Mac Catalyst uses UIKit and UIApplication, not AppKit and NSApplication. The targetEnvironment(macCatalyst) condition should be included in the first #if block with iOS/tvOS (line 3), not in the #elseif block with macOS (line 6). This causes the wrong framework to be imported and the wrong application type to be used for Mac Catalyst builds.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Use this label to trigger all PR workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logs: Batch Processor Minimize Data Loss

3 participants